[feat]: allow adding resource policy to dynamodb tables #154

shabbskagalwala · 2025-10-17T18:56:58Z

Issue #, if available: aws-controllers-k8s/community#2655

Description of changes:

Allows to add a resource policy to a dynamodb table.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ack-prow · 2025-10-17T18:57:10Z

Hi @shabbskagalwala. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

a-hilaly · 2025-10-17T18:57:40Z

/ok-to-test

a-hilaly

Thank you so much @shabbskagalwala ! this looks great overall - i left a few comments in line :)

a-hilaly · 2025-10-20T05:55:41Z

pkg/resource/table/hooks.go

+	if ackcompare.HasNilDifference(a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy) {
+		delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
+	} else if a.ko.Spec.ResourcePolicy != nil && b.ko.Spec.ResourcePolicy != nil {
+		if *a.ko.Spec.ResourcePolicy != *b.ko.Spec.ResourcePolicy {
+			delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
+		}
+	}
+


Is this block the same the original generated code when we set compare.is_ignore: false ?

Actually we do have a way to parse IAM policies that gets formatted/reformatted by the AWS APIs, checkout https://github.com/aws-controllers-k8s/iam-controller/blob/main/pkg/resource/role/hooks.go#L398-L432

a-hilaly · 2025-10-20T05:58:01Z

pkg/resource/table/hooks.go

 	}
-	if !delta.DifferentExcept("Spec.Tags") {
+
+	// ResourcePolicy can be updated independently of table state


pkg/resource/table/hooks_resource_policy.go

a-hilaly

@shabbskagalwala can we also get some basic e2e tests for this feature? a test case here would be great https://github.com/aws-controllers-k8s/dynamodb-controller/blob/main/test/e2e/tests/test_table.py

Co-authored-by: Amine <[email protected]>

a-hilaly · 2025-10-21T14:12:29Z

/retest

a-hilaly · 2025-10-21T16:25:59Z

/retest

shabbskagalwala · 2025-10-21T22:12:52Z

/retest

knottnt

@shabbskagalwala Thanks for the contribution!

test/e2e/tests/test_table.py

test/e2e/table.py

knottnt · 2025-10-22T20:26:53Z

pkg/resource/table/hooks_resource_policy.go

+	)
+	rm.metrics.RecordAPICall("DELETE", "DeleteResourcePolicy", err)
+	if err != nil {
+		var policyNotFoundErr *svcsdktypes.PolicyNotFoundException


p: It would be good to add a unit test validating this behavior.

ack-prow · 2025-10-22T20:50:30Z

@shabbskagalwala: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
dynamodb-verify-attribution	`fb7f703`	link	false	`/test dynamodb-verify-attribution`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

a-hilaly

Thank you 👍

a-hilaly · 2025-10-23T04:06:01Z

/lgtm

ack-prow · 2025-10-23T04:06:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, shabbskagalwala

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [a-hilaly]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…155) Issue #, if available: aws-controllers-k8s/community#2655 Description of changes: Release PR for #154 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ack-prow bot requested review from jlbutler and michaelhtm October 17, 2025 18:57

ack-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 17, 2025

ack-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 17, 2025

shabbskagalwala force-pushed the add-resource-policy branch 2 times, most recently from f868396 to 2266683 Compare October 17, 2025 19:30

[feat]: allow adding resource policy to dynamodb tables

7f28e30

shabbskagalwala force-pushed the add-resource-policy branch from 2266683 to 7f28e30 Compare October 17, 2025 19:31

a-hilaly reviewed Oct 20, 2025

View reviewed changes

Update pkg/resource/table/hooks_resource_policy.go

baa9115

Co-authored-by: Amine <[email protected]>

shabbskagalwala force-pushed the add-resource-policy branch 4 times, most recently from 78a6f96 to 2c79edd Compare October 21, 2025 04:48

shabbskagalwala force-pushed the add-resource-policy branch 2 times, most recently from 8aaf651 to 37b1875 Compare October 21, 2025 20:41

shabbskagalwala requested a review from a-hilaly October 22, 2025 15:03

knottnt reviewed Oct 22, 2025

View reviewed changes

[feat]: allow adding resource policy to dynamodb tables

fb7f703

shabbskagalwala force-pushed the add-resource-policy branch from 37b1875 to fb7f703 Compare October 22, 2025 20:49

a-hilaly approved these changes Oct 22, 2025

View reviewed changes

ack-prow bot added the approved label Oct 22, 2025

ack-prow bot assigned a-hilaly Oct 23, 2025

ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

ack-prow bot merged commit 888fee6 into aws-controllers-k8s:main Oct 23, 2025
6 of 7 checks passed

shabbskagalwala mentioned this pull request Oct 23, 2025

[feat]: release v1.6.0 - support resource policy on dynamodb tables #155

Merged

shabbskagalwala deleted the add-resource-policy branch October 23, 2025 04:16

[feat]: allow adding resource policy to dynamodb tables #154

[feat]: allow adding resource policy to dynamodb tables #154

Uh oh!

Conversation

shabbskagalwala commented Oct 17, 2025

Uh oh!

ack-prow bot commented Oct 17, 2025

Uh oh!

a-hilaly commented Oct 17, 2025

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

a-hilaly Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

a-hilaly Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

a-hilaly Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

a-hilaly commented Oct 21, 2025

Uh oh!

a-hilaly commented Oct 21, 2025

Uh oh!

shabbskagalwala commented Oct 21, 2025

Uh oh!

knottnt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

knottnt Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

ack-prow bot commented Oct 22, 2025

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

a-hilaly commented Oct 23, 2025

Uh oh!

ack-prow bot commented Oct 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants